Adjust static inline functions which don't inline #18454
Closed
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
These are found by adding
-Winline
on gcc. The flag is accepted on clang for compatibility with gcc, but it doesn't do anything.Mostly, this removes
inline
from the declarations. For local static functions, there is no point in addinginline
. This stems from either a misunderstanding of what it means, or alternatively is a holdover from ancient times where it may have meant something (note it still means something in a header file, but not a source file). A compiler is free to inline a static function, and specifying inline doesn't make the compiler inline the function. These pointlessinline
s make it harder to analyze failed inlining, so I remove them.However, for select functions, this changes them to
zend_always_inline
instead. This is because these functions didn't inline, but probably should for performance reasons. For instance,_class_exists_impl
is used in multiple frameless function handlers. I think in these cases, the authors were hoping they'd be inlined due to fact that are important for performance. There's also_php_search_array
, which is used with constant args on a variety of functions. It fails to inline due to its size, but if it did inline, it would cut large amounts of code out, so forcing the inlining seemed worth it.There's also
i_get_exception_base
which was removed. Again, the compiler is free to inlinezend_get_exception_base
if it wants to, andstatic inline
doesn't make the compiler inline it. This function often failed to inline in my-Winline
report. Alternatively this could be madezend_always_inline
, but since this is related to exceptions--which are supposed to be rare to begin with--it seemed more prudent to not force the compiler to inline it.This work is split out from #15075, so that's why it might seem like déjà vu.
There are still some
-Winline
failures, but they are in things I'd prefer not to change such as pcre or xxhash.